Skip to content

Don't create .msi installer for gnullvm hosts #141818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 10, 2025
Merged

Don't create .msi installer for gnullvm hosts #141818

merged 1 commit into from
Jun 10, 2025
+2 −1

Conversation

mati865
Copy link
Member

@mati865 mati865 commented May 31, 2025 •
edited
Loading

WIX toolset works only on Windows hosts, but gnullvm doesn't have host toolchain yet. To get out of this loop, we will create a single release without MSI installer.

Split out from: #140772

@rustbot
Copy link
Collaborator

rustbot commented May 31, 2025

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 31, 2025
@jieyouxu
Copy link
Member

jieyouxu commented May 31, 2025 •
edited
Loading

WIX toolset works only on Windows hosts.

If that's the case, then disabling it for non-Windows-hosts seems fine I think?

@mati865
Copy link
Member Author

mati865 commented May 31, 2025

Well, it disables creation of .msi installers that are officially provided by Rust. So I'm not sure whether it should be a more conscious decision (via the config) or the implication of using non-Windows host is fine.

@Noratrieb
Copy link
Member

I think there should be a config, it sounds surprising that you suddenly get different results with the same config.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 2, 2025

If the host was not Windows-based before, the build would fail (good luck running WIX or .exe files otherwise). Now it would be silently skipped. I don't think that we need a config for this, but the build should fail instead. So if target is Windows, but the host isn't, bootstrap should bail out.

@mati865
Copy link
Member Author

mati865 commented Jun 2, 2025

That would mean #140772 is a no-go in the current form.
I'll check how painful it would be to bootstrap from other Windows host soon.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 3, 2025 •
edited
Loading

Hmm, I don't see why that should be the case. Do the -gnullvm targets require MSI installers? If not, then we can just modify the check here from is_windows() to is_windows_but_not_gnu_llvm (or just is_msvc). If yes, then we cannot build them from a non-Windows host anyway.

@mati865
Copy link
Member Author

mati865 commented Jun 3, 2025

x.py dist for any Windows host does run (or at least tries to) WIX.
I had hoped msi installers could be missing for a single release (after the initial bootstrap these triples can host themselves in the dedicated job). So if condition like that is fine I'd go for it.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 3, 2025

Sorry, I'm not sure if I understand that 😅 If the -gnullvm targets should contain the MSI installer in their dist archives, we will need to build these target on Windows hosts, right? Because we can't produce these installers from Linux hosts (AFAIK?).

@mati865
Copy link
Member Author

mati865 commented Jun 3, 2025

In the end they should, to be consistent with other hosts at https://forge.rust-lang.org/infra/other-installation-methods.html
The question is whether gnullvm hosts could temporally skip .msi installer for their first release?
If the answer is no, or it requires a proposal, I'll drop this change and try with Windows jobs.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 3, 2025

The question is whether gnullvm hosts could temporally skip .msi installer for their first release?

I'm fine with that, but what I didn't understand is the "for their first release" part. What changes after the first release? How will we be able to create the MSI installers from Linux hosts?

@mati865
Copy link
Member Author

mati865 commented Jun 3, 2025

Sorry for not giving the full context here, I thought I had included it here in the previous comments, but it must have been a different issue/PR/channel.

Let me fix that.
My idea is to create the initial release with host tools for 86_64 and AArch64 windows-gnullvm triples using Linux host. It's the easiest way to achieve that (the possibility to run the containers locally helps a lot), but that means MSI installers are not achievable.
Once the host toolchain for these targets is present in the beta channel, Linux jobs for cross-compiling it would be replaced by native Windows jobs. Windows-gnullvm from that point forward would host themselves, so creating MSI installers becomes possible.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 4, 2025

Oh, I see, ok, thanks for the explanation. In that case I'm fine with that, although I would appreciate if the check was only scoped to the gnullvm targets, rather than windows as a whole.

@mati865 mati865 force-pushed the dont-create-msi-from-non-windows branch from 9f22e83 to 26415b3 Compare June 9, 2025 20:39
@mati865 mati865 changed the title Don't create .msi installer when not building from Windows Don't create .msi installer for gnullvm hosts Jun 9, 2025
@mati865 mati865 force-pushed the dont-create-msi-from-non-windows branch from 26415b3 to da90ea7 Compare June 9, 2025 21:46
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@mati865
Copy link
Member Author

mati865 commented Jun 9, 2025

@Kobzol sorry for the delay, I've been too busy recently. I think this should be fine to go now, once a new Rust version with gnullvm hosts is up, I'll revert this PR.

@mati865
Don't create .msi installer for gnullvm hosts
4f0b60a 
WIX toolset works only on Windows hosts and we need to boostrap this
host.
@mati865 mati865 force-pushed the dont-create-msi-from-non-windows branch from da90ea7 to 4f0b60a Compare June 9, 2025 21:54
@Kobzol
Copy link
Contributor

Kobzol commented Jun 10, 2025

Ok, thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 10, 2025

📌 Commit 4f0b60a has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2025
@fmease fmease mentioned this pull request Jun 10, 2025
Merged
bors added a commit that referenced this pull request Jun 10, 2025
@bors
Auto merge of #142299 - fmease:rollup-u86s80a, r=fmease
8ce2287 
Rollup of 16 pull requests

Successful merges:

 - #134442 (Specify the behavior of `file!`)
 - #140372 (Exhaustively handle parsed attributes in CheckAttr)
 - #140766 (Stabilize keylocker)
 - #141642 (Note the version and PR of removed features when using it)
 - #141818 (Don't create .msi installer for gnullvm hosts)
 - #141909 (Add central execution context to bootstrap)
 - #141992 (use `#[naked]` for `__rust_probestack`)
 - #142101 (core::ptr: deduplicate more method docs)
 - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods)
 - #142124 (Allow transmute casts in pre-runtime-MIR)
 - #142240 (deduplicate the rest of AST walker functions)
 - #142258 (platform-support.md: Mention specific Linux kernel version or later)
 - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`)
 - #142271 (compiler: fn ptrs should hit different lints based on ABI)
 - #142275 (rustdoc: Refractor `clean_ty_generics`)
 - #142288 (const_eval: fix some outdated comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 04025fa into rust-lang:master Jun 10, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 10, 2025
rust-timer added a commit that referenced this pull request Jun 10, 2025
@rust-timer
Unrolled build for #141818
f0e35c6 
Rollup merge of #141818 - mati865:dont-create-msi-from-non-windows, r=Kobzol

Don't create .msi installer for gnullvm hosts

WIX toolset works only on Windows hosts, but gnullvm doesn't have host toolchain yet. To get out of this loop, we will create a single release without MSI installer.

Split out from: #140772
@mati865 mati865 deleted the dont-create-msi-from-non-windows branch June 10, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Kobzol Kobzol

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
7 participants
@mati865 @rustbot @jieyouxu @Noratrieb @Kobzol @rust-log-analyzer @bors